Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added initial demos in gazebo_ros2_control_demos #2

Merged
merged 20 commits into from
Jul 28, 2020
Merged

Added initial demos in gazebo_ros2_control_demos #2

merged 20 commits into from
Jul 28, 2020

Conversation

ahcorde
Copy link
Collaborator

@ahcorde ahcorde commented May 28, 2020 •
edited
Loading

I have created this package for testing the gazebo_ros2_control. This package includes:

  • Launch files:
    • Launch Gazebo with a cart in a 30 meter rail
    • load the URDF based on a xacro file with the controllers controllers parameters,
    • robot_state_publisher
  • Config file
    • Config file with the type of controllers
  • Two examples that publish on follow_joint_trayectory

positionController

Signed-off-by: ahcorde ahcorde@gmail.com

@ahcorde
Added initial demos in gazebo_ros2_control_demos
189fb80 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde mentioned this pull request May 28, 2020
Closed
@ahcorde
Fixed install in CMakeLists.txt
9eeeca4 
Signed-off-by: ahcorde <ahcorde@gmail.com>
bmagyar
bmagyar requested changes May 28, 2020
View reviewed changes
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Just minor points:

  • Could you drop a few notes about reproducing this into a readme please? Which ROS2 distro, what was compiled from source?

  • Also, could you please decide whether to remove or uncomment the commented code in this PR?

@ahcorde
Copy link
Collaborator Author

ahcorde commented May 28, 2020

Could you drop a few notes about reproducing this into a readme please? Which ROS2 distro, what was compiled from source?

I added in this issue some instructions about how to reproduce it #3

I will update the README.md with more detailed instructions

@ahcorde
Removed commented code
9635877 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Added common ros 2 linters
961c25e 
Signed-off-by: ahcorde <ahcorde@gmail.com>
bmagyar
bmagyar approved these changes May 29, 2020
View reviewed changes
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together, this is a great start!

@ahcorde ahcorde requested a review from chapulina June 2, 2020 07:00
@ahcorde ahcorde mentioned this pull request Jun 3, 2020
Open
@ahcorde
Copy link
Collaborator Author

ahcorde commented Jun 3, 2020

Error is calculated for revolute joint not for others type of joints (continuous or prismatic). ros-controls/ros2_controllers#54

@chapulina
Copy link
Collaborator

I will update the README.md with more detailed instructions

+1, this would be very helpful. Both a top-level README explaining the packages in this repo, ROS version, etc, as well as a README per-package with quick instructions for each package.

@ahcorde
Added position pid example
7ac0bd7 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
Copy link
Collaborator Author

ahcorde commented Jun 22, 2020

Added position PID example

gazebo_ros2_control_position_pid

@ahcorde ahcorde self-assigned this Jun 22, 2020
@ahcorde ahcorde added the enhancement New feature or request label Jun 22, 2020
@ahcorde
clean config file
a54fc2f 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde mentioned this pull request Jun 22, 2020
Merged
chapulina
chapulina reviewed Jun 24, 2020
View reviewed changes
Copy link
Collaborator

@chapulina chapulina left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried a few demos with the instructions from #12 but they're not working for me

gazebo_ros2_control_demos/package.xml Show resolved Hide resolved
gazebo_ros2_control_demos/config/cartpole_controller.yaml Show resolved Hide resolved
@ahcorde
Copy link
Collaborator Author

ahcorde commented Jun 26, 2020

related to this PR #24

@ahcorde
Copy link
Collaborator Author

ahcorde commented Jul 9, 2020

need a rebase of this PR #28 when is merge on master

@chapulina
up to demos
927a59b 
Signed-off-by: Louise Poubel <louise@openrobotics.org>
chapulina
chapulina reviewed Jul 23, 2020
View reviewed changes
Copy link
Collaborator

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI is passing now, but I think it's because some exec dependencies are not declared.

gazebo_ros2_control_demos/package.xml Show resolved Hide resolved
@ahcorde
Included dependencies in the CMakeLists.txt
73a66bb 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde requested a review from chapulina July 24, 2020 08:27
@ahcorde
Copy link
Collaborator Author

ahcorde commented Jul 24, 2020

Pr to fix the CI is upstream ros-controls/ros2_controllers#81

chapulina
chapulina approved these changes Jul 27, 2020
View reviewed changes
Copy link
Collaborator

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I just have some small comments. I'd also suggest adding a README for this package, and also mentioning it on the top-level README.

gazebo_ros2_control_demos/launch/cart_example_position.launch.py Outdated Show resolved Hide resolved
gazebo_ros2_control_demos/launch/cart_example_position.launch.py Outdated
params = {'robot_description': robot_desc}

context = LaunchContext()
command = Command('xacro %s -o /tmp/test_cart_position.urdf' % xacro_file)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the /tmp directory feels a bit fragile. I wouldn't block this PR on this, but we should consider doing this some other way, maybe storing the URDF in a parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed a8ebd15

@ahcorde
avoid tmp files and removed comment
a8ebd15 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde
make linters happy
b01f47b 
Signed-off-by: ahcorde <ahcorde@gmail.com>
@ahcorde ahcorde merged commit 3b8d572 into master Jul 28, 2020
@ahcorde ahcorde deleted the ahcorde/initial/demos branch July 28, 2020 06:54
@ahcorde ahcorde mentioned this pull request Jul 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmagyar bmagyar bmagyar approved these changes

@chapulina chapulina chapulina approved these changes

Assignees

@ahcorde ahcorde

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ahcorde @chapulina @bmagyar